Skip to content

Add event publishing to core modules and improve framework testing#105

Merged
antosubash merged 5 commits intomainfrom
claude/review-framework-gaps-py59A
Apr 11, 2026
Merged

Add event publishing to core modules and improve framework testing#105
antosubash merged 5 commits intomainfrom
claude/review-framework-gaps-py59A

Conversation

@antosubash
Copy link
Copy Markdown
Owner

Summary

This PR addresses critical gaps in cross-module communication and testing infrastructure identified in the framework gap analysis. It adds event publishing to five core modules (Users, Products, FileStorage, PageBuilder, Settings), introduces comprehensive event bus integration tests, enhances validation capabilities, and improves runtime circular dependency detection.

Key Changes

Event Publishing & Cross-Module Communication

  • Users module: Added UserCreatedEvent, UserUpdatedEvent, UserDeletedEvent, UserRolesChangedEvent with publishing in UserService and UserAdminService
  • Products module: Added ProductCreatedEvent, ProductUpdatedEvent, ProductDeletedEvent with publishing in ProductService
  • FileStorage module: Added FileUploadedEvent, FileDeletedEvent with publishing in FileStorageService
  • PageBuilder module: Added PageCreatedEvent, PagePublishedEvent, PageUnpublishedEvent, PageDeletedEvent with publishing in PageBuilderService
  • Settings module: Added SettingChangedEvent, SettingDeletedEvent with publishing in SettingsService

Testing Infrastructure

  • EventBusIntegrationTests: New comprehensive integration test suite verifying cross-module event flows with the full DI container, including:
    • Single and multiple handler invocation
    • Exception isolation (one handler failure doesn't prevent others from running)
    • Background event dispatch with BackgroundEventDispatcher
    • Pipeline behavior wrapping
  • TestEventBus: New test stub for unit tests to record published events without wiring the full pipeline
  • WebApplicationFactoryTests: Added timeout-based circular dependency detection for contract resolution and event bus publishing, catching runtime cycles that SM0010 cannot detect

Authorization & Permissions

  • UsersPermissions: New permission class with View, Create, Update, Delete operations
  • SettingsPermissions: New permission class with View, Update, Manage operations
  • MarketplacePermissions: New permission class with View, Install operations
  • Module registration updated to include IModulePermissions implementations

Validation Enhancements

  • ValidationBuilder: Extended with fluent string validation methods:
    • Required(), MaxLength(), MinLength(), LengthBetween()
    • MatchesPattern() with compiled regex support
    • Email() with generated regex pattern
    • GreaterThan(), Between() for numeric validation

Framework Infrastructure

  • ModuleHealthCheck: New health check aggregating status from all discovered modules via IModule.CheckHealthAsync()
  • ContractRegistryEmitter: New source generator emitter creating ModuleContractRegistry for test enumeration of all contract interfaces
  • Lazy: Added to DI to break factory-lambda circular dependencies (e.g., SettingsService ↔ AuditingEventBus)
  • HealthCheckConstants: Added ModulesCheckName constant

Frontend Improvements

  • Network error handling: Added explicit handling for network exceptions and offline/online state transitions
  • Toast system refactor: Generalized showErrorToast() to showToast() supporting error, warning, and success variants with configurable auto-dismiss and close callbacks
  • Offline detection: Window-level listeners for offline/online events with persistent offline toast until reconnection

Documentation

  • CONSTITUTION.md: Corrected diagnostic numbering for SM0045-SM0054 (Feature Flags, Endpoints, Module Metadata sections)
  • framework-gaps-review.md: Comprehensive gap analysis document identifying 28 framework gaps across 7 categories with severity levels and remediation guidance

Notable Implementation Details

  • Event handlers are registered via DI and discovered through IEventHandler<T> interface, enabling automatic handler discovery
  • Exception handling in event publishing uses AggregateException to isolate handler failures while allowing other handlers to execute
  • Validation builder uses Regex.IsMatch() static method which benefits from .NET's internal regex cache for common patterns
  • Health checks run module checks in parallel for performance
  • Contract registry generation only includes interfaces with exactly one implementation, matching DI registration behavior

https://claude.ai/code/session_01PRezWVEFAC7Gzx88WQJ4qA

claude added 5 commits April 10, 2026 17:10
Review of all 23 modules, core architecture, source generator diagnostics,
frontend, testing infrastructure, and security patterns. Identifies 28 gaps
across 7 categories with priority-ranked recommendations.
P1 — High Impact:
- Add domain events to Products, Users, FileStorage, PageBuilder,
  Settings modules (ProductCreated/Updated/Deleted, UserCreated/
  Updated/Deleted/RolesChanged, FileUploaded/Deleted, PageCreated/
  Published/Unpublished/Deleted, SettingChanged/Deleted)
- Wire up IEventBus publishing in each service's create/update/
  delete flows, using PublishAsync for creates/updates and
  PublishInBackground for deletes
- Add EventBusIntegrationTests covering cross-module flows
  (handlers, pipeline behaviors, partial failures, background
  dispatch) — 5 new tests
- Fix circular dependency: SettingsService resolves IEventBus
  lazily via IServiceProvider, breaking the cycle created by
  AuditingEventBus decorator which itself depends on
  ISettingsContracts

P2 — Important:
- Add permissions classes and ConfigurePermissions wiring for
  Settings, Users, and Marketplace modules
- Extend ValidationBuilder with Required, MinLength, MaxLength,
  LengthBetween, MatchesPattern, Email, GreaterThan, Between
  helpers
- Add ModuleHealthCheck that aggregates per-module CheckHealthAsync
  results into a single aggregate health check registered alongside
  DatabaseHealthCheck
- Add network error handling to frontend app.tsx: online/offline
  detection via window events, persistent offline toast, network
  exception handler on router
- Document structured logging conventions and database migration
  strategy in Constitution
- Fix CLAUDE.md diagnostic range (SM0001-SM0054)
- Restructure Constitution diagnostic sections to match actual
  source generator implementation (Feature Flags, Endpoints,
  Module Metadata)
The circular dependency that hung every integration test
(SettingsService → IEventBus → AuditingEventBus → ISettingsContracts)
was invisible to the source generator's SM0010 check because:

1. SM0010 only sees module-level dependencies derived from project
   references, not runtime DI graphs.
2. AuditingEventBus is registered via a factory lambda
   (AddScoped<IEventBus>(sp => new AuditingEventBus(...))) — static
   analysis cannot reliably look inside lambda bodies to see the
   ISettingsContracts dependency.
3. .NET's built-in DI cannot track re-entry through factory
   lambdas, so the cycle manifests as infinite recursion / hang
   rather than a clean InvalidOperationException.

Add runtime defenses:

- New ContractRegistryEmitter generates ModuleContractRegistry.All
  listing every discovered contract interface with exactly one
  registered implementation (mirrors the existing
  ModuleDbContextRegistry pattern).

- New theory test ContractInterface_CanBeResolved_WithoutHanging
  iterates ModuleContractRegistry.All and resolves each contract
  in a fresh scope with a 5-second timeout.

- New fact EventBus_CanBeResolved_WithoutHanging resolves IEventBus
  directly (the decorator chain's focal point).

- New fact EventBus_CanPublishEvent_WithoutHanging publishes a test
  event end-to-end to catch cycles that only manifest during handler
  dispatch.

All tests run the resolution on a thread-pool thread with
Task.WhenAny against a Task.Delay, because GetService is synchronous
and cannot be cancelled. A hang fails fast with a clear message
pointing at the IEventBus → AuditingEventBus → ISettingsContracts
pattern.

Verified the tests fail correctly when the bug is reintroduced:
14/28 contract resolutions hang, and both IEventBus tests fail
within 11 seconds with the diagnostic message.
Code reuse:
- Lift `TestEventBus` into `SimpleModule.Tests.Shared/Fakes/` so the
  5 module test projects stop duplicating an identical nested class.
  The canonical fake now records `PublishedEvents` so tests can
  assert on publishing behaviour.
- Wire up the existing but unreferenced `WriteHealthCheckResponse`
  on the `/health/ready` endpoint.

Code quality:
- `ModuleHealthCheck`: replace fragile `.Replace("Module", "")`
  name extraction with `ModuleAttribute.Name`, cache the
  `(module, name)` map once in the constructor, and drop the
  dead `IModuleLifecycle` branch (the interface does not hide
  `IModule.CheckHealthAsync`).
- `EventBusIntegrationTests`: collapse three byte-identical
  `TrackingHandler` classes into one generic
  `TrackingHandler<TSlot>` with empty marker types, drop the
  unused `AnotherEvent` and `WasCalled` derived property, and
  replace the polling loop in the background-dispatch test with
  a deterministic `TaskCompletionSource` + `WaitAsync`.
- `WebApplicationFactoryTests`: delete the 14-line narration
  block; leave a single short "why" comment about SM0010 not
  catching factory-lambda cycles. Replace the manual
  `Task.WhenAny(task, Task.Delay(...))` timeout pattern with
  `Task.WaitAsync(timeout)` + `TimeoutException` catch.
- `app.tsx`: collapse three near-identical `showErrorToast`/
  `showPersistentWarningToast`/`showSuccessToast` functions
  into a single `showToast({ variant, title, message, ... })`
  helper driven by a `toastStyles` map.
- `SettingsService`: replace the `IServiceProvider` service-
  locator anti-pattern with `Lazy<IEventBus>` to break the
  `SettingsService ↔ AuditingEventBus ↔ ISettingsContracts`
  cycle cleanly. A new framework-level registration provides
  `Lazy<IEventBus>` out of the box.
- `HealthCheckConstants.ModulesCheckName` constant replaces the
  raw `"modules"` string literal.

Efficiency:
- `ModuleHealthCheck`: dispatch to all modules in parallel via
  `Task.WhenAll` instead of sequential `await` in a foreach.
  Readiness probes called by load balancers no longer pay the
  sum of all module latencies.
- `UserAdminService.SetUserRolesAsync`: drop the redundant
  `GetRolesAsync` round-trip after the diff — the new role set
  is already in memory.
- `ValidationBuilder.MatchesPattern`: pass `RegexOptions.Compiled`
  so the static regex cache stores compiled patterns.

Verified: `dotnet build` clean (0/0), `dotnet test` passes
all 1208 tests across 24 projects, `biome check` clean.
…rk-gaps-py59A

# Conflicts:
#	modules/FileStorage/tests/SimpleModule.FileStorage.Tests/FileStorageServiceTests.cs
#	modules/Settings/src/SimpleModule.Settings/SettingsService.cs
@cloudflare-workers-and-pages
Copy link
Copy Markdown

Deploying simplemodule-website with  Cloudflare Pages  Cloudflare Pages

Latest commit: fd66bcd
Status: ✅  Deploy successful!
Preview URL: https://4f7e1d2b.simplemodule-website.pages.dev
Branch Preview URL: https://claude-review-framework-gaps.simplemodule-website.pages.dev

View logs

@antosubash antosubash merged commit f64a585 into main Apr 11, 2026
5 checks passed
@antosubash antosubash deleted the claude/review-framework-gaps-py59A branch April 11, 2026 22:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants